Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clock driver for meson #278

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Add clock driver for meson #278

wants to merge 11 commits into from

Conversation

terryzbai
Copy link

@terryzbai terryzbai commented Nov 1, 2024

This clock driver configures all clocks as needed at boot time. The initial configurations are parsed from device tree with a python script (which will replaced with either Zig or Rust) and saved as a C header file to be accessed by clock driver.

The design of clock driver is derived from Linux Common Clock Framework (CCF) that unifies client interfaces and key data structures but simplified for modularity and maintainability. Additionally, clock definitions and operations are implemented as static to avoid dynamic memory allocation and other dangerous behaviours.

The current implementation assumes that all client requests are safe and compatible with the existing configurations, especially for clk_set_rate(), which simply adjust clock rates or propagate one level up to parent clock without any checking. A virtualiser is probably needed to manage requests according to the safety policy. determine_rate() and round_rate() need to be implemented to consider more complicated cases, such as multiple clock consumers requesting different frequencies, and dynamically clock scaling for power management.

Linux clock driver is the main reference but it is not always compatible with what described in the datasheet, and it could go through some obscure code somewhere, so there might be some unknown errors in my code.

The clock driver for maaxboard is in progress and coming soon.
The implementation for maaxboard is also ready.

@terryzbai terryzbai force-pushed the clk_driver branch 2 times, most recently from 642cb84 to d53cb39 Compare November 1, 2024 06:28
@terryzbai terryzbai marked this pull request as draft November 4, 2024 06:32
Copy link
Contributor

@wom-bat wom-bat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are too close to the Linux drivers. We don't need to provide a run-time interface for different clocks, as any sDDF system runs only on a single SoC --- so get rid of all the ..._ops structures and call the functions directly.

Plus please make sure all lines are under 80 chars wide, and add copyright and SPDX Licence tags where they;re missing. Also remember to sign-off your commit messages.

#include <sddf/timer/client.h>
#include <sddf/util/printf.h>

int reg_write(uint64_t base, uint32_t offset, uint32_t val)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make static inline and in header

return 0;
}

int regmap_update_bits(uint64_t base, uint32_t offset, uint8_t shift, uint8_t width, uint32_t val)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make static inline and in header

return 0;
}

int regmap_read_bits(uint64_t base, uint32_t offset, uint8_t shift, uint8_t width)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make static inline; maybe should be uint32_t rather than int

return reg_val;
}

int regmap_mux_update_bits(uint64_t base, uint32_t offset, uint8_t shift, uint32_t mask, uint32_t val)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably abstract all the regmap_xxx_update functions into a single set instead of having separate mux and ordinary ones

return reg_val;
}

static int clk_gate_enable(struct clk *clk)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be static inline

@@ -0,0 +1,230 @@
/*
*
* This program is derived from:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add SPDX licence id

return regmap_read_bits(base, parm.reg_off, parm.shift, parm.width);
}

/* TODO: Replace this doggy dealy() with a standard interface */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delay not dealy

int i;
for (i = 0; i < num_regs; i++) {
reg_write(base, regs[i].reg, regs[i].def);
/* TODO: delay is needed */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this TODO not already done?

regmap_update_bits(clk->base, data->sdm.reg_off, data->sdm.shift, data->sdm.width, sdm);
regmap_update_bits(clk->base, data->n2.reg_off, data->n2.shift, data->n2.width, n2);

/* volatile uint32_t *clk_reg = ((void *)clk_base + data->sdm.reg_off); */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

clk->hw.init->ops->set_rate(clk, req_rate, prate);
*rate = req_rate;
return 0;
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Else not needed

This implementation includes all clock definitions, basic operations
of clock compoents, as well as interfaces to interact with clicnets
via PPC. Client should be able to enable/disable a specific gate
clock or query the current clock rate by passing the corresponding
clock identifier, which is listed in g12a-bindings.h.

set_rate() assumes that the rate requst is valid and compatible with
the current configurations. Complete implementation that considers
more incompatible requests will come in the future when dynamic
clock configuration is necessary in lionsos

Signed-off-by: Terry Bai <[email protected]>
Signed-off-by: Terry Bai <[email protected]>
Add clock driver for maaxboard. This commit includes all clock component
definitions, basic operations, and a simple test on maaxboard. Scripts are
added in build systems. A few files are reorganised for reuse

Signed-off-by: Terry Bai <[email protected]>
Signed-off-by: Terry Bai <[email protected]>
This commit includes modifiation to the interfaces, primarily for set/get rate, which
return error state as well as final rate.

More tests are conducted to verify the basic operations of types of clock components.
A few bugs are fixed as well.

Signed-off-by: Terry Bai <[email protected]>
@terryzbai terryzbai force-pushed the clk_driver branch 2 times, most recently from 160af1b to 3ec98a8 Compare November 18, 2024 01:25
This commit fixed style and license issues

Signed-off-by: Terry Bai <[email protected]>
@terryzbai terryzbai force-pushed the clk_driver branch 2 times, most recently from b13868e to 2007cd7 Compare November 18, 2024 05:29
This commit replaces all platform-dependent data types with fix-sized
types, and unifies the return values of interfaces as signed integers.
The return value 0 indicates the success on the request to clock driver,
and negative values indicate the failure reasons.

Signed-off-by: Terry Bai <[email protected]>
Abstract a set of regmap_* operations by combining mux-specific ones and ordinary ones

Signed-off-by: Terry Bai <[email protected]>
Add clk_get_parent() and clk_set_parent() interfaces for the clients.
Fixed several logic issues in related implementations. More clock
component definition checkings and fixup.

Signed-off-by: Terry Bai <[email protected]>
This commit adds set_rate() implementations for maaxboard. Now clocks can
be set to target rate at initialisation time and configured dynamically.

Signed-off-by: Terry Bai <[email protected]>
This commit reorgnises source files to allow more code to be shared
for multiple boards.

Refactor makefiles:
- move objectives to clk_driver directory
- use built-in CFLAGS in rules

Temporary fixup for a zig build issue: create_clk_config.py cannot find
the lazypath `zig-out/bin` when executing. The current solution is to
create an install directory in advance, but it would be a bit of trouble
to find the generated header file if someone want to read or modify it.

Moved clock specifier binding files to `include/sddf/clk`, so the clients
will be able to configure clocks with macro constants

Signed-off-by: Terry Bai <[email protected]>
@terryzbai terryzbai marked this pull request as ready for review November 21, 2024 01:44
@terryzbai terryzbai requested a review from wom-bat November 21, 2024 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants